Skip to content

fix(ollama): make native num_ctx opt-in#2589

Merged
benfrank241 merged 3 commits into
vectorize-io:mainfrom
r266-tech:r266/ollama-native-num-ctx-config
Jul 9, 2026
Merged

fix(ollama): make native num_ctx opt-in#2589
benfrank241 merged 3 commits into
vectorize-io:mainfrom
r266-tech:r266/ollama-native-num-ctx-config

Conversation

@r266-tech

Copy link
Copy Markdown
Contributor

Summary

  • stop sending native Ollama num_ctx unless it is explicitly configured
  • add HINDSIGHT_API_LLM_OLLAMA_NUM_CTX and thread it through config, LLMProvider, and MemoryEngine
  • document the migration path for operators who want to keep the old num_ctx=16384 request behavior
  • add regression coverage for env parsing, direct-constructor validation, positional timeout compatibility, and native Ollama request options

Fixes #2572.

Tests

  • python3 -m py_compile hindsight-api-slim/hindsight_api/config.py hindsight-api-slim/hindsight_api/engine/llm_wrapper.py hindsight-api-slim/hindsight_api/engine/memory_engine.py hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py hindsight-api-slim/tests/test_config_validation.py hindsight-api-slim/tests/test_llm_wrapper.py
  • ruff check hindsight-api-slim/hindsight_api/config.py hindsight-api-slim/hindsight_api/engine/llm_wrapper.py hindsight-api-slim/hindsight_api/engine/memory_engine.py hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py hindsight-api-slim/tests/test_config_validation.py hindsight-api-slim/tests/test_llm_wrapper.py
  • ruff format --check hindsight-api-slim/hindsight_api/config.py hindsight-api-slim/hindsight_api/engine/llm_wrapper.py hindsight-api-slim/hindsight_api/engine/memory_engine.py hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py hindsight-api-slim/tests/test_config_validation.py hindsight-api-slim/tests/test_llm_wrapper.py

Targeted pytest did not run in this local environment: uv run cannot install onnxruntime==1.27.0 for macOS arm64, and the system Python environment is missing project deps such as asyncpg.

@benfrank241

Copy link
Copy Markdown
Member

really nice bug report and fix — the repro and the ollama ps eviction trace made this trivial to verify. removing the hardcoded num_ctx=16384 and making it opt-in is exactly right, and the validation + regression tests are solid.

i pushed one small commit (da12fd93) adding HINDSIGHT_API_LLM_OLLAMA_NUM_CTX to .env.example (+ the byte-identical hindsight-embed copy) — new config flags need to land there per our checklist, and the env-template sync test stays green. hope you don't mind me pushing directly.

one thing to confirm before i merge: you're threading it from config.llm_ollama_num_ctx rather than per-member/per-bank, so it's static-global. i think that's the right call — context window is a property of the ollama host, not a per-bank preference — but it does diverge from the member.* pattern in that same ensemble block, so just confirm that's intentional and i'll merge.

@r266-tech

Copy link
Copy Markdown
Contributor Author

Yes, that scope is intentional.

For the native Ollama path this PR changes, the override is a process-level config value: config.llm_ollama_num_ctx is threaded into LLMProvider / MemoryEngine and then used by _call_ollama_native(). It is not a member or bank field.

The behavior I intended and covered here is:

  • unset env: do not send num_ctx, so Ollama's Modelfile/server/model default wins
  • HINDSIGHT_API_LLM_OLLAMA_NUM_CTX=16384: preserve the old request-forced value for operators who want that migration path

I chose that scope because the bug was caused by Hindsight forcing a request-level Ollama runner option even when the Ollama deployment already had its own context configuration. If a deployment needs different context sizes per Ollama host/model, I think the safer path is server-side Modelfile settings or separate Hindsight processes rather than one shared process env var.

I also pushed cc207113 to keep the new config field optional for direct HindsightConfig construction (kw_only default None) and added a small regression for that compatibility case. Local py_compile passed for the touched config/test files; CI has started on the new head.

Thanks for adding the env-template entries and keeping the generated-files check green.

@benfrank241 benfrank241 merged commit e29ee58 into vectorize-io:main Jul 9, 2026
87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ollama native provider hardcodes num_ctx=16384, overriding Modelfile context and causing reload/eviction loop

2 participants